Conversation
| @@ -77,7 +78,7 @@ module.exports = { | |||
| break; | |||
| default: | |||
| util.error("Unknown operatation"); | |||
There was a problem hiding this comment.
Maybe you could fix this spelling error while you're here
| @@ -41,7 +42,7 @@ module.exports.dispatcher = function (args) { | |||
| break; | |||
| default: | |||
| util.error("Unknown operatation"); | |||
There was a problem hiding this comment.
another "operatation" :)
|
Didn't we want to use different error codes for different errors? I thought that was the idea when we added the gap between testjob errors and testengine errors. Perhaps it's not needed, I don't know. |
I think we decided to use only these codes to start with. |
| util.error("Unknown operatation"); | ||
| break; | ||
| util.error("Unknown operation"); | ||
| process.exit(100); |
There was a problem hiding this comment.
I think this error state is neither a testengine failure nor a testjob error. We kinda repurpose the 100-code a bit. I don't think it is super important, but I think someone else might. Could be worth asking about it. Especially since we say that "unknown operation" is a '1' later.
There was a problem hiding this comment.
Yes, this is not related to testjob and testengine. May be it should not be exited with error code. Instead just give the message as before
| default: | ||
| util.error("Unknown operatation"); | ||
| break; | ||
| util.error("Unknown operation"); |
| default: | ||
| util.error("Unknown operatation"); | ||
| break; | ||
| util.error("Unknown operation"); |
There was a problem hiding this comment.
This is a recurring pattern, maybe a function "exitWithStatusAndErrorMessage" would save a few lines in the code.
I think we wanted to craete a division between testengine errors and tool errors. |
…er to test status codes and see if something is broken. Updated all status codes to be '1' and added them to a few places to ensure everything works properly.
…the testScript and ensure that we return the correct status codes whenever we return print the usage.
| case 'import': | ||
| if (args.length < 2) { | ||
| util.error("Usage: testengine user import <file/url>"); | ||
| util.printErrorAndExit("Usage: tes tengine user import <file/url>"); |
| default: | ||
| util.error("Unknown operatation"); | ||
| break; | ||
| util.printErrorAndExit("Unknown operation") |
| @@ -0,0 +1,121 @@ | |||
| import os | |||
There was a problem hiding this comment.
I am guessing this test script will run all the combination of commands. But may be its good have some in the readme on how to run this command
There was a problem hiding this comment.
I think if we want to document it a comment at the top of this file is better, I think it might be better to write it in JS as well.
There was a problem hiding this comment.
would be nice to have it in Js
… describes how to run it. Fixing minor errors.
lootic
force-pushed
the
TSERV-861-return-error-code
branch
from
b39a5ed to
25f2abf
Compare
|
|
||
| function startSlowJobAndThenRunCli(command, flag) { | ||
| let testJobId = startJob('slow.xml') | ||
| runCli(command, flag, testJobId) |
There was a problem hiding this comment.
semicolon missing. There are other places as well where semicolon is missing. Please take a look
4 participants
No description provided.